-
Notifications
You must be signed in to change notification settings - Fork 752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[main] Update dependencies from dotnet/aspnetcore #4061
[main] Update dependencies from dotnet/aspnetcore #4061
Conversation
…0230609.2 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23307.20 -> To Version 8.0.0-preview.6.23309.2 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23280.5 -> To Version 8.0.0-preview.6.23308.1 (parent: Microsoft.AspNetCore.App.Runtime.win-x64
Interesting error from the broken tests:
Could it be related to dotnet/runtime#87067? cc @stephentoub |
This method was introduced there. Are you trying to run on a newer aspnetcore than runtime? |
Oh okay that makes sense then. Yeah we are updating the version of runtime we compile against, but not the one that we run against, so it makes sense for this to be breaking test execution. I'll update the SDK used by the repo to one that includes the change in order to fix this. |
…0230610.1 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23307.20 -> To Version 8.0.0-preview.6.23310.1 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23280.5 -> To Version 8.0.0-preview.6.23308.17 (parent: Microsoft.AspNetCore.App.Runtime.win-x64
…0230610.2 Microsoft.AspNetCore.App.Runtime.win-x64 , Microsoft.AspNetCore.Mvc.Testing , Microsoft.AspNetCore.TestHost , Microsoft.Extensions.Caching.StackExchangeRedis , Microsoft.Extensions.Diagnostics.HealthChecks , Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions , Microsoft.Extensions.Features , Microsoft.Extensions.Http.Polly , Microsoft.Extensions.ObjectPool From Version 8.0.0-preview.6.23307.20 -> To Version 8.0.0-preview.6.23310.2 Dependency coherency updates Microsoft.Bcl.TimeProvider,Microsoft.Extensions.Caching.Abstractions,Microsoft.Extensions.Caching.Memory,Microsoft.Extensions.Configuration.Abstractions,Microsoft.Extensions.Configuration.Binder,Microsoft.Extensions.Configuration.CommandLine,Microsoft.Extensions.Configuration.Json,Microsoft.Extensions.Configuration,Microsoft.Extensions.DependencyInjection.Abstractions,Microsoft.Extensions.DependencyInjection,Microsoft.Extensions.Hosting.Abstractions,Microsoft.Extensions.Hosting,Microsoft.Extensions.Http,Microsoft.Extensions.Logging.Abstractions,Microsoft.Extensions.Logging.Configuration,Microsoft.Extensions.Logging.Console,Microsoft.Extensions.Logging,Microsoft.Extensions.Options.ConfigurationExtensions,Microsoft.Extensions.Options.DataAnnotations,Microsoft.Extensions.Options,Microsoft.Extensions.Primitives,System.Collections.Immutable,System.Configuration.ConfigurationManager,System.Diagnostics.DiagnosticSource,System.Diagnostics.PerformanceCounter,System.IO.Hashing,System.Net.Http.Json,System.Security.Cryptography.Pkcs,System.Security.Cryptography.Xml,System.Text.Encodings.Web,System.Text.Json,System.Runtime.Caching From Version 8.0.0-preview.6.23280.5 -> To Version 8.0.0-preview.6.23309.7 (parent: Microsoft.AspNetCore.App.Runtime.win-x64
…pp3.1 builds which depend on a dotnet 7 era ILLink package
@@ -7,6 +7,7 @@ | |||
<add key="dotnet-eng" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json" /> | |||
<add key="dotnet-tools" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json" /> | |||
<add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" /> | |||
<add key="dotnet7" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7/nuget/v3/index.json" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding this? We shouldn't have any dependencies on .NET 7 feed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vitek-karas could you help us understand why we have .NET 7 dependency for .NET Core 3.1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to me that this file gets generated here: https://github.com/dotnet/installer/blob/f8275a0edc0a90b593f2e3f73c4b9b3e908e3e22/src/redist/targets/GenerateBundledVersions.targets#L989-L991
Which uses this version:
That is why we ended up having to add the dependency to the .net 7 feed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sbomer would know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change now causes downstream repos to stop building unless those add a dependency on dotnet7 feed (which is a very undesirable thing):
D:\a\_work\1\s\src\Microsoft.Azure.Extensions.Messaging.StorageQueues\Microsoft.Azure.Extensions.Messaging.StorageQueues.csproj : error NU1603: Microsoft.Azure.Extensions.Messaging.StorageQueues depends on Microsoft.NET.ILLink.Analyzers (>= 7.0.100-1.23211.1) but Microsoft.NET.ILLink.Analyzers 7.0.100-1.23211.1 was not found. An approximate best match of Microsoft.NET.ILLink.Analyzers 8.0.100-1.22608.1 was resolved. [TargetFramework=net8.0]
##[error]src\Microsoft.Azure.Extensions.Messaging.StorageQueues\Microsoft.Azure.Extensions.Messaging.StorageQueues.csproj(0,0): error NU1603: (NETCORE_ENGINEERING_TELEMETRY=Build) Microsoft.Azure.Extensions.Messaging.StorageQueues depends on Microsoft.NET.ILLink.Analyzers (>= 7.0.100-1.23211.1) but Microsoft.NET.ILLink.Analyzers 7.0.100-1.23211.1 was not found. An approximate best match of Microsoft.NET.ILLink.Analyzers 8.0.100-1.22608.1 was resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the file @joperezr pointed to says that when targeting netcoreapp3.1, we need to use the net7.0 version of this package (targeting net7.0 has the same requirement). There's normally some process to keep the 7.0 versions required by the 8.0 SDK in sync - so that the 8.0 SDK depends on 7.0 servicing versions released at a similar cadence.
What seems to have gone wrong here is that the 7.0 packages didn't get published. I checked that the 7.0.304 SDK uses the 7.0.100-1.23211.1 version of ILLink.Tasks, but it hasn't been published for use by the 8.0 SDK. I will follow up with the folks responsible for publishing these packages offline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @sbomer. Should the packages be published to dotnet8 (or perhaps dotnet-public) feed, or is there an expectation that consumers will be required to pull from dotnet7 feed?
We can't pull from non-managed feeds (like nuget.org).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcpopMSFT what's the normal location for downlevel packages that the SDK depends on when targeting downlevel TFMs? Do those go to the dotnet-public feed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may of course be wrong here so I'll wait for @marcpopMSFT to answer the above question, but IMO any packages that are produced by .NET and may be required by the .NET 8 SDK and is not already in the dotnet-public feed (meaning it's not yet in NuGet), then I believe it should be in the dotnet8 feed. It doesn't "feel right" to have to depend on the dotnet7 feed when using the latest .NET 8 SDK.
This pull request updates the following dependencies
Coherency Updates
The following updates ensure that dependencies with a CoherentParentDependency
attribute were produced in a build used as input to the parent dependency's build.
See Dependency Description Format
From https://github.com/dotnet/aspnetcore
Microsoft Reviewers: Open in CodeFlow